Skip to content

fix: keep stepped range progress totals aligned#9582

Open
GHX5T-SOL wants to merge 3 commits into
marimo-team:mainfrom
GHX5T-SOL:fix/9575-progress-bar-range-step
Open

fix: keep stepped range progress totals aligned#9582
GHX5T-SOL wants to merge 3 commits into
marimo-team:mainfrom
GHX5T-SOL:fix/9575-progress-bar-range-step

Conversation

@GHX5T-SOL
Copy link
Copy Markdown

This pull request was authored by a coding agent.

Summary

  • Fix mo.status.progress_bar(range(...)) when total is inferred, so stepped ranges track the number of yielded items instead of advancing past the inferred total.
  • Preserve existing explicit-total behavior for stepped ranges, where range.step is used with the caller-provided total.
  • Add regressions for both inferred and explicit totals on range(0, 10, 2).

Closes #9575.

Validation

  • uv run --group test pytest tests/_plugins/stateless/status/test_progress.py -q (15 passed)
  • uv run ruff check marimo/_plugins/stateless/status/_progress.py tests/_plugins/stateless/status/test_progress.py
  • uv run ruff format --check marimo/_plugins/stateless/status/_progress.py tests/_plugins/stateless/status/test_progress.py
  • uv run --only-group typecheck mypy marimo/_plugins/stateless/status/_progress.py
  • git diff --check
  • git diff | gitleaks stdin --no-banner --redact --timeout 30

Limitations

  • I ran focused Python/status validation only; I did not run the full test suite or a browser UI smoke.
  • I did not take any account/legal action for the CLA beyond reading the contribution guidance.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment May 22, 2026 8:14am

Request Review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 18, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Architecture diagram
sequenceDiagram
    participant User as User Code
    participant PB as progress_bar()
    participant PR as ProgressBar <br/>_progress.py
    participant PBar as Progress <br/>bar component
    
    User->>PB: progress_bar(range(0, 10, 2))
    PB->>PR: __init__(collection=range, total=None)
    PR->>PR: total_was_provided = (total is not None)
    PR->>PR: total is None → infer total
    
    alt No explicit total
        PR->>PR: total = len(range) = 5
        PR->>PR: collection is range →<br/>total_was_provided is False →<br/>skip step override
        PR->>PR: step remains default 1
    else Explicit total provided
        User->>PB: progress_bar(range(0, 10, 2), total=10)
        PB->>PR: __init__(collection=range, total=10)
        PR->>PR: total_was_provided = True
        PR->>PR: total = 10 (caller-provided)
        PR->>PR: collection is range AND<br/>total_was_provided is True →<br/>step = collection.step = 2
    end
    
    PR-->>PB: ProgressBar instance
    PB-->>User: ProgressBar instance
    
    Note over User,PBar: Iteration
    
    loop Over range(0, 10, 2)
        User->>PR: next()
        PR->>PBar: update(step=1)
        PBar-->>PR: current += 1
        PR-->>User: [0, 2, 4, 6, 8]
    end
    
    Note over User,PBar: Final state differs by branch
    
    alt No explicit total
        User->>PR: check progress.current, progress.total
        PR-->>User: (5, 5) — tracks yielded items
    else Explicit total
        User->>PR: check progress.current, progress.total
        PR-->>User: (10, 10) — step applied to caller's total
    end
Loading

Re-trigger cubic

@GHX5T-SOL GHX5T-SOL marked this pull request as ready for review May 18, 2026 19:47
@mscolnick
Copy link
Copy Markdown
Contributor

@GHX5T-SOL, please sign the CLA

@GHX5T-SOL
Copy link
Copy Markdown
Author

recheck

@GHX5T-SOL
Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

)

if isinstance(collection, range):
if total_was_provided and isinstance(collection, range):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, but this could be an elif

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this to an elif in 45db08c. Thanks.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Bundle Report

Changes will increase total bundle size by 95.79kB (0.38%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
marimo-esm 25.26MB 95.79kB (0.38%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: marimo-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/cells-*.js 3.16kB 709.93kB 0.45%
assets/JsonOutput-*.js 195.69kB 556.64kB 54.21% ⚠️
assets/index-*.js -176.19kB 431.38kB -29.0%
assets/index-*.css 949 bytes 366.91kB 0.26%
assets/dist-*.js 33 bytes 137 bytes 31.73% ⚠️
assets/dist-*.js 2 bytes 104 bytes 1.96%
assets/dist-*.js -8 bytes 169 bytes -4.52%
assets/dist-*.js 23 bytes 160 bytes 16.79% ⚠️
assets/dist-*.js -119 bytes 137 bytes -46.48%
assets/dist-*.js 27 bytes 164 bytes 19.71% ⚠️
assets/dist-*.js 172 bytes 276 bytes 165.38% ⚠️
assets/dist-*.js 79 bytes 183 bytes 75.96% ⚠️
assets/dist-*.js -7 bytes 176 bytes -3.83%
assets/dist-*.js 171 bytes 335 bytes 104.27% ⚠️
assets/dist-*.js -20 bytes 256 bytes -7.25%
assets/dist-*.js 243 bytes 403 bytes 151.88% ⚠️
assets/dist-*.js -65 bytes 104 bytes -38.46%
assets/dist-*.js -7 bytes 169 bytes -3.98%
assets/dist-*.js 8 bytes 177 bytes 4.73%
assets/dist-*.js -231 bytes 104 bytes -68.96%
assets/dist-*.js -285 bytes 102 bytes -73.64%
assets/dist-*.js -16 bytes 387 bytes -3.97%
assets/edit-*.js 78 bytes 325.17kB 0.02%
assets/reveal-*.js 72.54kB 249.59kB 40.98% ⚠️
assets/layout-*.js 499 bytes 198.73kB 0.25%
assets/dependency-*.js 341 bytes 156.71kB 0.22%
assets/file-*.js 195 bytes 47.03kB 0.42%
assets/input-*.js 46 bytes 60.48kB 0.08%
assets/panels-*.js 680 bytes 47.89kB 1.44%
assets/react-*.browser.esm-Ce2ksurd.js (New) 25.64kB 25.64kB 100.0% 🚀
assets/state-*.js -13 bytes 24.79kB -0.05%
assets/download-*.js 5 bytes 9.48kB 0.05%
assets/scratchpad-*.js 15 bytes 8.4kB 0.18%
assets/config-*.js 315 bytes 6.41kB 5.17% ⚠️
assets/ttcn-*.js 7 bytes 64 bytes 12.28% ⚠️
assets/ttcn-*.js -7 bytes 57 bytes -10.94%
assets/useBoolean-*.js -2.75kB 3.02kB -47.66%
assets/slides-*.css 224 bytes 529 bytes 73.44% ⚠️
assets/react-*.browser.esm-CgWOEYeG.js (Deleted) -25.64kB 0 bytes -100.0% 🗑️

@dmadisetti dmadisetti disabled auto-merge May 21, 2026 20:24
@GHX5T-SOL GHX5T-SOL force-pushed the fix/9575-progress-bar-range-step branch from 22807be to e7d0193 Compare May 22, 2026 08:13
@GHX5T-SOL
Copy link
Copy Markdown
Author

Rebased this branch on current main at e7d01930f after the unrelated Py 3.14 scratchpad integration job failed on the stale head. The PR diff is still limited to the progress-bar range-step fix and its focused tests.

Validation run locally:

  • uv run --group test pytest tests/_plugins/stateless/status/test_progress.py -q -> 15 passed
  • git diff --check origin/main...HEAD

I also tried the failing scratchpad integration test locally, but the local server fixture timed out before the test body because static assets are not built in this checkout, so the GitHub Actions rerun is the meaningful signal for that job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mo.status.progress_bar should use the step property of 'range'

3 participants